node:tls: report a wrapped socket's connect failure on the TLSSocket by robobun · Pull Request #38122 · oven-sh/bun

robobun · 2026-08-13T08:32:30Z

Problem

tls.connect({ socket }) over a net.Socket whose connect is still in flight (net.connect(...) passed straight in, or a new net.Socket() connected after being wrapped) never reports a failed connect: the plain socket emits error + close, the TLSSocket emits nothing, and a caller waiting on its secureConnect / error / close hangs.
If the caller only listens on the TLSSocket (the documented usage), the plain socket's ECONNREFUSED is an uncaught exception instead. Same after tls.connect({ socket }).destroy() while the socket was still connecting.
The connect listener also stays armed after the failure: a net.Socket that is reconnected afterwards (retry logic reusing the socket) is upgraded to TLS by the dead wrap, so a plain server gets a ClientHello and the orphaned TLSSocket reports ERR_SSL_WRONG_VERSION_NUMBER.
Cause: the wait-for-connect arm of Socket.prototype.connect (src/js/node/net.ts, around line 2030) only registers connection.once("connect", ...). Nothing links the two sockets until that upgrade runs, so the connection's error / close go nowhere. The already-connected arm (upgradeTLSDeferred) and the duplex arm (upgradeDuplexToTLS wires close) do not have this gap.
Node prints raw error ECONNREFUSED, tls error ECONNREFUSED, raw close, tls close for the repro; bun printed only the two raw lines.

Fix

While waiting for connect, also listen for the connection's error (re-emitted on the TLSSocket through _emitTLSError) and close (destroys the TLSSocket). connect removes both listeners before upgrading; close removes the connect listener, which is what stops the dead wrap from upgrading a later reconnect.
Matches node: _init does wrap.on('error', err => this._emitTLSError(err)) and _wrapHandle does wrap.on('close', () => this.destroy()) (lib/internal/tls/wrap.js L977 / L740 in v26.3.0). Going through _emitTLSError keeps node's _controlReleased semantics, and the resulting events (error carrying the connection's own error object, then close with hadError === false) are exactly node's.
The listeners are scoped to the waiting period because after the upgrade the adopted native handle (or the duplex engine) already reports the connection's errors and close to the TLSSocket itself, and a net.Socket can be reconnected after it closes, so listeners left behind would act on a TLSSocket that has moved on.
error is ignored once the TLSSocket is already destroyed: bun keeps the connection connecting until connect fires after tls.connect({ socket }).destroy(), and its failure must not surface as an error after the TLSSocket's close. The listener still absorbs it, so it is no longer an uncaught exception either.
Complementary to node:tls: make client-side new TLSSocket(socket) upgrade the socket it wraps #37664 / node:tls: destroy the wrapped net.Socket when tls.connect({ socket }) is destroyed #34507, which tear the wrapped socket down when the TLSSocket is destroyed (the other direction); neither makes a failed connect reach the TLSSocket. One more neighbour: a client TLSSocket built with new tls.TLSSocket(socket) and then explicitly connect({ socket })ed keeps the stream as its _handle, and destroy() on such a socket throws handle.close is not a function on main today (node:tls: handle destroy() on a TLSSocket wrapping an unconnected stream #35842 fixes that); tls.connect({ socket }), the path this PR is about, has no _handle while waiting.
Verified with the five new tests in test/js/node/tls/node-tls-connect.test.ts (describe("tls.connect({ socket }) over a net.Socket whose connect has not completed yet")): connect in flight, connect started after wrapping with listeners on the TLSSocket only, raw.destroy() while connecting, tlsSocket.destroy() while connecting, and reconnecting the socket after the failure. All five fail without the src change (two time out, two die on the unhandled ECONNREFUSED, the reconnect one gets ERR_SSL_WRONG_VERSION_NUMBER; checked with USE_SYSTEM_BUN=1 for all five and with a bun bd build of main for the first four), pass with the fix, and stayed green over repeated runs.
The rest of node-tls-connect.test.ts, node-tls-connect-hostname-verification.test.ts, node-http-connect.test.ts's https CONNECT case (wraps a still-connecting socket and expects the handshake to succeed), and the vendored test-tls-net-socket-keepalive, test-tls-connect-given-socket, test-socket-writes-before-passed-to-tls-socket, test-tls-connect-stream-writes, test-tls-socket-close, test-tls-socket-destroy, test-tls-connect-pipe, test-tls-connect-simple, test-tls-connect-abort-controller, test-tls-socket-allow-half-open-option, test-tls-socket-failed-handshake-emits-error pass. node-net.test.ts has the same 12 failures with and without the change (this container resolves localhost to ::1 for the server and 127.0.0.1 for the client; node behaves the same here).

Background

tls.connect({ socket }) builds a TLSSocket over a transport the caller owns instead of opening its own TCP connection. In bun the TLS layer is attached in Socket.prototype.connect, in one of three ways: a connected net.Socket has its native handle adopted on the spot (upgradeTLSDeferred), a generic duplex / named pipe gets a stream-level TLS engine (upgradeDuplexToTLS), and a net.Socket that has no handle yet or is still connecting waits for its connect event and then takes one of the first two paths. Only that third, waiting state had no link between the two sockets.
_emitTLSError is node's routing for errors that happen on a TLSSocket's underlying transport: it emits _tlsError and, once _releaseControl() has handed the socket to the user (which tls.connect() does right away), emits error. It does not destroy the socket; in node the socket is destroyed by the transport's close, which is why the TLSSocket's close carries hadError === false in this flow.

Repro and node / bun output

import net from "node:net";
import tls from "node:tls";
const srv = net.createServer().listen(0, "127.0.0.1", () => {
  const port = srv.address().port;
  srv.close(() => {
    const raw = net.connect({ port, host: "127.0.0.1" }); // still connecting when wrapped
    raw.on("error", e => console.log("raw error", e.code));
    raw.on("close", hadError => console.log("raw close", hadError));
    const c = tls.connect({ socket: raw, rejectUnauthorized: false });
    c.on("error", e => console.log("tls error", e.code));
    c.on("close", hadError => console.log("tls close", hadError));
  });
});

node v26.3.0 and bun with this change:

raw error ECONNREFUSED
tls error ECONNREFUSED
raw close true
tls close false

bun 1.4.0 (and a debug build of main):

raw error ECONNREFUSED
raw close true

The same probe with raw.destroy() right after wrapping prints raw close false / tls close false on node and with this change, only raw close false before. With c.destroy() right after wrapping and no listeners on raw, bun before this change exits with an uncaught ECONNREFUSED; node and bun with this change print tls close false and exit cleanly.

tls.connect({ socket }) over a net.Socket that has not connected yet only waited for the socket's 'connect' before upgrading it. If that connect failed, the plain socket emitted 'error' and 'close' but the TLSSocket never emitted anything, so callers waiting on it hung. Node re-emits the wrapped socket's 'error' on the TLSSocket and destroys the TLSSocket when the wrapped socket closes; do the same while waiting for 'connect', and drop the listeners once the upgrade happens.

coderabbitai · 2026-08-13T08:32:39Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f649717-cbd6-40f7-aebb-62a471fd727e

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 9c8e832.

📒 Files selected for processing (2)

src/js/node/net.ts
test/js/node/tls/node-tls-connect.test.ts

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-08-13T08:32:45Z

^{Updated 2:38 PM PT - Aug 13th, 2026}

✅ @robobun, your commit 9c8e832106d8afac57a1903601520e1227c9946a passed in Build #94795! 🎉

🧪 To try this PR locally:

bunx bun-pr 38122

That installs a local version of the PR into your bun-38122 executable, so you can run:

bun-38122 --bun

robobun · 2026-08-13T08:41:51Z

Status: reproduced and fixed, waiting on CI.

Reproduced on bun 1.4.0 and a debug build of main with the repro in the PR description: tls.connect({ socket: net.connect(closedPort) }) prints only the plain socket's error / close; node also prints them on the TLSSocket. With the caller listening on the TLSSocket alone, the plain socket's ECONNREFUSED is an uncaught exception instead, and a socket reconnected after the failure gets upgraded to TLS by the dead wrap (ERR_SSL_WRONG_VERSION_NUMBER).
Fix is in Socket.prototype.connect's wait-for-connect arm (src/js/node/net.ts): the connection's error is re-emitted through _emitTLSError and its close destroys the TLSSocket; every listener is removed once connect or close fires.
Tests: the describe at the end of test/js/node/tls/node-tls-connect.test.ts (five cases); all of them fail without the src change and pass with it.

claude

LGTM — focused Node compat fix wiring the wrapped socket's error/close to the waiting TLSSocket.

What was reviewed:

Traced listener lifecycle: onConnect removes error+close before upgrading; onClose removes connect+error then destroys; onError is absorbed (no re-emit) once the TLSSocket is already destroyed — no leaked listeners on any path.
Confirmed _emitTLSError is on TLSSocket.prototype and _releaseControl() runs in tls.connect before these can fire, so the error reaches user listeners.
Checked the four new tests: hermetic (127.0.0.1, refusedPort() holds a live local port so no listen(0) race), await the actual close, and cover connect-in-flight / connect-after-wrap / raw.destroy() / tlssock.destroy() variants.

Extended reasoning...

Overview

Adds error and close listeners on the wrapped net.Socket for the wait-for-connect arm of Socket.prototype.connect in src/js/node/net.ts (~22 lines), so a failed underlying connect reaches the TLSSocket instead of leaving it pending or surfacing as an uncaught exception. Four new tests in test/js/node/tls/node-tls-connect.test.ts assert the exact event sequences node v26.3.0 produces.

Security risks

None. This is error/close event forwarding during the pre-handshake window; it does not touch certificate verification, rejectUnauthorized, trust stores, or any handshake logic. The only new call is to the existing _emitTLSError, which routes through node's _tlsError/_controlReleased semantics already in tls.ts.

Level of scrutiny

Moderate — TLS-adjacent networking code, but the change is a small, additive event-listener wiring in a single well-understood branch. The already-connected (upgradeTLSDeferred) and duplex (upgradeDuplexToTLS) arms are untouched; the PR description correctly notes those already have equivalent linkage. I traced each path: on connect the new listeners are removed before the (unchanged) upgrade runs; on close the connect/error listeners are removed and this.destroy() is called (idempotent if already destroyed); onError is guarded by !this.destroyed so a post-destroy connect failure is absorbed but not re-emitted, matching node and the fourth test. The arrow functions correctly bind this to the TLSSocket, and this branch is only reachable when bunTlsSymbol is a function (so _emitTLSError exists on the prototype).

Other factors

Tests follow the repo's guidance: no sleeps, await the observable close, wire failure paths explicitly (closed() helper avoids events.once rejecting on the expected error), await using disposes the port holder. The refusedPort() helper avoids the bind-then-release race by using a held ephemeral local port.
PR description reports all four tests fail on main / with USE_SYSTEM_BUN=1 and pass with the fix over 10 repeated runs, plus a broad set of adjacent tls/net tests remain green.
No CODEOWNERS entry for src/js/node/net.ts. Matches local style (plain .on/.once/.removeListener, same as neighboring lines 1999–2002).

claude

Automated review ran and found no bugs. The fix is small and well-tested, but it adds interacting event listeners to the TLS wrap's wait-for-connect state machine in src/js/node/net.ts — a human look at the socket-lifecycle interactions would still be worthwhile.

Checked: listener add/remove pairing (onConnect/onClose each remove the others; .once self-removes), _emitTLSError exists on TLSSocket.prototype and matches node's _controlReleased gating, this.destroy() in onClose is idempotent when the TLSSocket was already destroyed, and .on (not .once) for error matches node's _init. The five new tests cover the variant matrix (connect-in-flight, connect-after-wrap, raw.destroy(), client.destroy(), reconnect-after-failure) and each awaits an actual close rather than sleeping.

Extended reasoning...

Overview

The PR fixes tls.connect({ socket }) when the wrapped net.Socket is still connecting: previously only a connect listener was registered on the underlying connection, so a failed connect (ECONNREFUSED) never reached the TLSSocket — callers waiting on secureConnect/error/close would hang, and the plain socket's error could become an uncaught exception. The fix adds error (routed through _emitTLSError) and close (destroys the TLSSocket) listeners for the waiting period, each removing the others when it fires. This mirrors node's internal/tls/wrap.js _init/_wrapHandle. Two files touched: ~15 net lines in src/js/node/net.ts:2030-2091 and five new tests in test/js/node/tls/node-tls-connect.test.ts.

Security risks

None identified. The change is error-propagation plumbing during the pre-handshake wait; it doesn't touch certificate verification, rejectUnauthorized, or any crypto path. The onError guard (if (!this.destroyed)) prevents emitting on a dead socket rather than suppressing anything security-relevant.

Level of scrutiny

Moderate-to-high. src/js/node/net.ts is core networking infrastructure that underpins TLS/HTTP/HTTPS. The change itself is small and the listener add/remove pairing checks out, but socket-lifecycle state machines in the TLS wrap are subtle — the interaction between onConnect removing onClose, onClose removing onConnect, and the this.destroyed guard in onError deserves a look from someone with deep knowledge of bun's upgrade paths (upgradeTLSDeferred / upgradeDuplexToTLS). Not a mechanical config/typo change.

Other factors

The PR description is unusually thorough: cites the exact node source lines being mirrored, explains why listeners are scoped to the waiting period (post-upgrade the native handle already reports errors), and documents which existing tests were re-run.
Test coverage is strong: five cases covering connect-in-flight, connect-after-wrap, raw.destroy(), client.destroy(), and socket reuse after failure. Tests await real close events (not sleeps), use a refusedPort() helper that holds a live connection's local port to guarantee ECONNREFUSED, and assert exact event sequences including hadError values matched against node v26.3.0.
The comment-cop bot's feedback about long comments was addressed in 9c8e832 and both threads are resolved.
No CODEOWNERS entry covers these paths.
Verified _emitTLSError exists on TLSSocket.prototype (src/js/node/tls.ts:853) and its semantics (emits _tlsError, then error if _controlReleased) match what the PR description claims.

Builds on #42176, which is merged. The base of this PR is `main`. Consolidates #42240 (the same fix). ### Problem - A TLS upgrade over a generic `Duplex` listens for the transport's `data`, `end`, `drain` and `close`, but not its `error`. So `transport.destroy(err)` after the upgrade throws `err` as an uncaught exception. Node emits it on the TLS socket. - `transport.listenerCount('error')` is 0 in bun and 1 in node v26.3.0. An `https.request` over such a socket kills the process and does not fail the request. ### Fix - `forwardUpgradedError` (`src/js/node/net.ts`) forwards the transport's `error` through `_emitTLSError`. The four stream-engine attach sites call it. A client sees `'error'`. A server wrap keeps it on `'_tlsError'` (`'tlsClientError'` on a `tls.Server`). The transport's `'close'` then ends the socket (#42176). - A `net.Socket` transport (a named pipe, unflushed writes, TLS over TLS) is excluded. Its close paths synthesize a read `ECONNRESET` as soon as anything listens for `'error'`. See Notes. - Verified: `test/js/node/tls/node-tls-connect.test.ts`, 5 new tests. One runs `node-tls-duplex-transport-error-fixture.ts` through `bunRun` (six wrap cases in one subprocess). All 5 fail on `main`. Also `test/js/node/tls` and 9 vendored `test-tls-*` tests. ### Background - A stream with no fd cannot use the kernel TLS path. `upgradeDuplexToTLS` runs a BoringSSL engine over the stream, driven by four native thunks on the stream's events. None is an error thunk. - Node wraps the stream in a `JSStreamSocket`, which re-emits the stream's `'error'`. `TLSSocket._init` forwards that error with `_emitTLSError`. - `_emitTLSError` already exists in `src/js/node/tls.ts`. It emits `'_tlsError'` always, and `'error'` only once `_releaseControl` has run. `tls.connect()` releases control at once, a server socket on `'secure'`. <details><summary>Notes</summary> #### Measurements Each case wraps a `Duplex`, then calls `transport.destroy(new Error("transport failed"))`. Events on the TLS socket (or the request), linux x64, debug+ASAN. The first six rows are `node-tls-duplex-transport-error-fixture.ts`, and the node column is v26.3.0 with the same fixture. The `https.request` row runs in the test process. | case | `main` (has #42176) | this branch | node v26.3.0 | | --- | --- | --- | --- | | `tls.connect({ socket })`, destroy in the same tick | uncaught `transport failed`, non-zero exit | `_tlsError`, `error`, `close:false` | same | | `tls.connect({ socket })`, destroy after the ClientHello | uncaught, non-zero exit | `_tlsError`, `error`, `close:false` | same | | server wrap, destroy in the same tick | uncaught, non-zero exit | `_tlsError`, `close:false` | same | | server wrap, destroy after the engine started | uncaught, non-zero exit | `_tlsError`, `close:false` | same | | `tlsServer.emit("connection", duplex)`, destroy in the same tick | uncaught, non-zero exit | `tlsClientError` with the `TLSSocket`, `close:false` | same | | `tlsServer.emit("connection", duplex)`, destroy after the engine started | uncaught, non-zero exit | `tlsClientError` with the `TLSSocket`, `close:false` | same | | `https.request` over the wrap | uncaught, non-zero exit | `req.error`, `req.close` with `req.destroyed === true` | same | A transport that errors without closing also matches node: the error reaches the TLS socket and the socket stays alive (`destroyed === false`). Two more cases from #42240, measured by hand on this branch (no test): | case | bun 1.4.3-canary.1 | this branch | node v26.3.0 | | --- | --- | --- | --- | | `tlsSocket.destroy()` over `Duplex.from({ readable, writable })` | uncaught `ABORT_ERR` | `error:ABORT_ERR`, `close:false` | same | | transport whose `write` calls back with an error, `s.write(data, cb)` queued | uncaught `EBOOM`, `cb` never called, no `'close'` | `error:EBOOM`, `cb(ERR_SOCKET_CLOSED)`, `close:false` | `error:EBOOM`, `cb(ECANCELED)`, `close:false` | The callback code in the last row differs because node cancels the queued write through `JSStreamSocket.doClose`. #35386 covers `ECANCELED` for cancelled TLS writes. #### Consolidation of #42240 #42240 carried the same listener with the same `net.Socket` guard. This branch keeps the test shape the review here asked for (`bunRun`, a fixture file, a one-line comment plus the node links). The cases that only #42240 had are now in the fixture: a server wrap at both timings, `https.request` over the wrap, `'_tlsError'`, and the `'close'` `hadError` flag. #42176 dropped the `attachUpgradedDuplex` helper that both PRs had patched. It now attaches the four thunks inline at each site. So the listener moved to `forwardUpgradedError`, which each site calls after it attaches the four thunks. #### Scope: every `net.Socket` transport is left out Two arms wrap a `net.Socket`, and neither gets the listener: - The fd-adoption arm. `tls.connect({ socket: connectedNetSocket })` with no queued writes hands the fd to a native TLS pair (`upgradeTLS`) and attaches nothing to the `net.Socket`. - The stream-level engine over a `net.Socket`: TLS over TLS, a named pipe, or a socket with queued plain writes. These reach `forwardUpgradedError` and the `instanceof Socket` check skips them. Node attaches the same listener in both, so `sock.destroy(err)` on such a transport is still uncaught in bun. The reason is one mechanism. `SocketEmitEndNT`, `SocketHandlers2.close`, `failWrite` and `ServerHandlers.error` turn a peer reset into `destroy(ECONNRESET)` only when the socket has an `'error'` listener, and stay silent otherwise. A forwarder counts as a listener. The first push of #42240 attached it to every transport. `test-tls-inception.js` (TLS over TLS, no `'error'` listener anywhere) then failed on Windows x64, Windows arm64 and macOS arm64 with an uncaught `read ECONNRESET`. On Windows x64 the base passed 5 of 5 runs, that push failed 5 of 5, and the narrowed listener passed 6 of 6. Those arms also have a second route to the same failure (the raw socket's own dispatch), so they need the de-duplication that #36534 and #38122 are about. `new tls.TLSSocket(stream)` with no `connect()` wraps nothing today, so nothing reaches it there either. That is #37664's subject. `_http2_upgrade.ts` has a fifth copy of the same four `.on()` calls. #38124 routes its transport error to the h2 session. #### Fixture startup The fixture runs its six cases in one subprocess and takes the key and the cert from `KEY` and `CERT`, which the test sets. An earlier shape ran five subprocesses at once, and each imported `harness`. A debug build needs about 1.5 s to load `node:tls` and about 1 s more for `harness`, so on a loaded machine (load average 150 on 16 cores) two of the five reached the 5 s test timeout. The one subprocess takes 2.1 to 2.8 s on the same machine. #### Suites `test/js/node/tls/node-tls-connect.test.ts`: 84 pass, 0 fail. `test/js/node/tls/`: 391 pass, 3 fail, none on the Duplex wrap path. `SNICallback runs even when the requested servername matches the bind hostname` binds `localhost` and connects to `127.0.0.1`, and fails the same way on the released binary in this container. `tls.Server socket destroySoon > delivers the whole stream when destroySoon follows end` (64 rounds of 2 MB) times out at 5 s on this debug build, and does the same with `main`'s `net.ts`. `root certificate initialization > concurrent Workers all see the same CA certificate lists` times out in some runs on this loaded machine. Vendored, all exit 0: `test-tls-inception`, `test-tls-js-stream`, `test-tls-connect-given-socket`, `test-tls-delayed-attach-error`, `test-tls-socket-failed-handshake-emits-error`, `test-tls-over-http-tunnel`, `test-tls-starttls-server`, `test-tls-destroy-stream`, `test-https-agent-create-connection`. </details>  --- **[human-review]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 7 failed, 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [4.73ms] (pass) should thow ECONNRESET if FIN is received before handshake [534.04ms] (pass) initializes authorizationError to null in the TLSSocket constructor [6.87ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [120.38ms] (pass) should be able to grab the JSStreamSocket constructor [14.03ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [116.64ms] (pass) tls.connect > should have peer certificate when using self asign certificate [150.83ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port ... (truncated) release without fix: 11 failed, 18 skipped bun test v1.4.3-canary.1 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [0.03ms] (pass) should thow ECONNRESET if FIN is received before handshake [6.17ms] (pass) initializes authorizationError to null in the TLSSocket constructor [0.16ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [3.30ms] (pass) should be able to grab the JSStreamSocket constructor [0.21ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [5.48ms] (pass) tls.connect > should have peer certificate when using self asign certificate [4.83ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port and host correctly (skip) tls.connect > should process port, host, and callback correctly (skip) tls.connect > should handle the absence of a callback gracefully (skip) tls.c ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [3.79ms] (pass) should thow ECONNRESET if FIN is received before handshake [386.28ms] (pass) initializes authorizationError to null in the TLSSocket constructor [6.71ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [102.58ms] (pass) should be able to grab the JSStreamSocket constructor [15.93ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [97.47ms] (pass) tls.connect > should have peer certificate when using self asign certificate [122.29ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port a ... (truncated) release with fix: 18 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 9098c05 features lto, baseline 23 deps, 136 codegen, 1176 objects in 977ms ninja: Entering directory `/workspace/bun/build/release' [1/4] fetch lolhtml [lolhtml] up to date [2/4] fetch rust-argon2 [rust-argon2] up to date [2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json 244 units: 172 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib [3/4] reconfigure [1/1499] mkdir codegen [2/1499] mkdir stamps [3/1499] install /workspace/bun bun install v1.4.3-canary.1 (367d939) Checked 26 installs across 65 packages (no changes) [15.00ms] [4/1499] install /workspace/bun/packages/bun-error bun install v1.4.3-canary.1 (367d939) Checked 1 install across 2 packages (no changes) [4.00ms] [5/1499] install /workspace/bun/src/node-fallbacks bun install v1.4.3-canary.1 (367d939) Checked 111 installs across 104 packages (no changes) [6.00ms] [6/1499] rustc unicode_ident [7/1499] gen node-fallbacks ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/js/node/net.ts | 10 ++++ test/js/node/tls/node-tls-connect.test.ts | 45 ++++++++++++++++- .../tls/node-tls-duplex-transport-error-fixture.ts | 58 ++++++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) ``` </details> **gate history** · 5 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/js/node/net.ts 13 12 41 test/js/node/tls/node-tls-connect.test.ts 3 4 39 …/js/node/tls/node-tls-duplex-transport-error-fixture.ts 1 3 44 ``` </details>  --------- Co-authored-by: robobun <robobun@bun.sh> Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>

@Write

…t wraps (#37664) ### Problem - `new tls.TLSSocket(socket)` on the client side (STARTTLS) does nothing on main. A write throws `TypeError: socket.@Write is not a function`. `_start()` throws `ERR_MISSING_ARGS`. - Since #42181 (not released), `end()` throws an uncaught `TypeError: socket.shutdown is not a function` at `endNT (node:net)`. - Cause: the constructor stored the wrapped stream as `_handle` (`src/js/node/tls.ts`). Nothing replaced it with a TLS handle. ### Fix - The constructor runs the upgrade of `tls.connect({ socket })` (`kUpgradeClientTLS`, `src/js/node/net.ts`). `_handle` is never the stream. `_start()` is a no-op. - The wrap completes like node's `_finishInit`: `'secure'` and `ssl.verifyError()`. It gets no hostname check and no `'secureConnect'`, and `authorized` stays `false`. - Verified: `test/js/node/tls/node-tls-connect.test.ts`. 16 of its 21 new tests fail on main. Also `test/js/node/tls/` and 657 vendored node tests. ### Background - STARTTLS changes a plaintext connection to TLS in place. The `mysql` driver 2.18.1 does it with this constructor. No user filed an issue for it. - In `node:net`, `_write`, `_final` and `_destroy` call into `_handle` as a native handle. - Only `tls.connect()` adds node's `onConnectSecure` (hostname check, `authorized`, `'secureConnect'`). A wrap gets `_finishInit` only. - Considered a start on `_start()`, as in node. Each handle call then needs a guard. ### Downsides - An unused wrap now sends a ClientHello of 1450 bytes (main and node: 0). `setServername()` and `setSession()` after construction have no effect on that handshake. - Unlike node, a wrap rejects an untrusted certificate unless the caller passes `rejectUnauthorized: false`. An app that does its own check must pass `false`. <details><summary>Notes</summary> **Scope.** This head is the core only, as the review of 2026-09-24 asked. The same review decided that a wrap rejects an untrusted certificate by default. Two parts of the earlier head are gone, because other changes own them. #42235 landed the forwarding of the `'error'` of a `Duplex`, and #43791 owns it for a `net.Socket`. #38028 owns the destroy of a wrapped socket that has not connected yet. The `UpgradedDuplex.rs` hunk landed with #36909. The review of 2026-09-25 asked for three more changes: commits 6657bc3 and ae669bf, and this body. The review of 2026-09-30 asked for one more: commit a460a9d. **Changes since the earlier head that the review did not list.** - The `open` handler of an upgraded socket applies the `session` option on the native socket. See "Sessions" below. - A wrap gives the `NODE_TLS_REJECT_UNAUTHORIZED=0` warning of `tls.connect()`. - `authorized` and `authorizationError` keep their initial values on a wrap, as in node. The earlier head set `authorized = true` for a good chain. The wrap checks no host name, so that value accepted a certificate of any host. The verdict is `ssl.verifyError()`. - A wrap does not get `onConnectEnd`. A peer that closes during the handshake gives `'end'`, `'finish'`, `'close'` and no `ECONNRESET`, as in node. `tls.connect({ socket })` keeps its `ECONNRESET`. - `servername` is passed into the upgrade. Before, it reached the ClientHello only when the caller gave no `secureContext`. - `kStandaloneWrap` is initialised in the `Socket` constructor. **Differences from node v26.3.0 that stay.** The review kept the start of the handshake in the constructor. The `mysql` driver, the main user of this API, calls `_start()` right after the constructor, so it sees no difference. | shape | node | this PR | | --- | --- | --- | | wrap that is never used | sends nothing | sends a ClientHello (1450 bytes) | | `setServername()` after the constructor | applies, the handshake starts later | too late. Pass `servername` as an option. | | `setSession()` after the constructor | applies | no effect. Pass `session` as an option. | | untrusted certificate, `rejectUnauthorized` absent or `true` | `'secure'`, the caller must read `ssl.verifyError()` | destroy with the verify error, `'_tlsError'`, no `'secure'` | | untrusted certificate, `rejectUnauthorized: false` | `'secure'`, data flows | the same | | `new TLSSocket(raw)`, then `tls.connect({ socket: raw })` | `EALREADY` on the wrap | `Invalid socket` on the `tls.connect` client (main: works, because its wrap does nothing) | Node never rejects a certificate on a wrap. It leaves the check to the app, so an app that forgets the check accepts any certificate. Here the wrap uses the rule of `tls.connect()`: it rejects unless the caller passes `rejectUnauthorized: false`, or `NODE_TLS_REJECT_UNAUTHORIZED` is `0`. Before commit 4d8b9e5, only `rejectUnauthorized: true` rejected, and a wrap with default options accepted each certificate, as in node. The `mysql` driver listens to `'secure'` and to `'_tlsError'`. If the wrap emitted `'secure'` and then destroyed itself, the driver would report a bad chain two times. The replay test asserts one report. **Sessions.** BoringSSL's `SSL_set_session` calls `abort()` when the handshake has started, in release builds too. `TLSSocket.prototype.setSession()` calls the native function at once, on main and on this head. #41671 puts the guard in the native function, for each caller: a late `setSession()` then throws `Already started.`. An earlier head of this PR made `setSession()` only store the session. The review asked to take that rule out, and commit ae669bf did. For a client-side wrap the review then asked for one narrow rule, in commit a460a9d: the `TLSSocket` constructor sets `kStandaloneWrap`, and `setSession()` returns at once when it is set. | shape | node | main | this PR | | --- | --- | --- | --- | | `new TLSSocket(raw)`, `setSession()`, `_start()` | resumes | throws `ERR_MISSING_ARGS` | no effect, full handshake | | `tls.connect({ socket })`, then `setSession()` | no effect | process aborted | process aborted | | `setSession()` inside `'secureConnect'` | no effect | process aborted | process aborted | | `tls.connect({ port })`, then `setSession()` before it connects | resumes | resumes | resumes | | `tls.connect({ port })`, then `setSession()` in the `'connect'` listener | full handshake | resumes | resumes | | `tls.connect({ socket, session })` | resumes | full handshake | resumes | | `new TLSSocket(raw, { session })` | resumes | throws `ERR_MISSING_ARGS` | resumes | In the first row, the wrap has sent its ClientHello when `setSession()` runs. Without the check in `setSession()`, that row aborts the process (exit 134). The rule also holds for a wrap over a socket that is still connecting: node resumes there, and this PR runs a full handshake. The last two rows come from one hunk that stays. `SocketHandlers2.open` applies the `session` option on the native socket. An fd upgrade assigns `_handle` after `open`, so `self.setSession()` dropped the option there. **Two bugs of main that this PR does not fix. An open PR owns each.** - The native `setSession()` has no check of the handshake state. `socket.setSession()` in the `handshake` callback of a `Bun.connect` socket aborts the process (exit 134, release build of main). #41671 fixes it in `set_session` in `src/runtime/socket/tls_socket_functions.rs`, for each caller. It makes a late `setSession()` throw `Already started.`. - Over a `Duplex`, TLS inside TLS, or a named pipe, a handshake that fails is reported as success. A peer that answers the ClientHello with plaintext gives `'secureConnect'` for `tls.connect({ socket: duplex, rejectUnauthorized: false })` on main, and `'secure'` for a wrap with `rejectUnauthorized: false` here. Node gives `ERR_SSL_WRONG_VERSION_NUMBER`. Over a TCP socket the result is correct. The stream engine in `src/uws/lib.rs` reports no protocol error. #32929 fixes it there. One more door of the same bug: with `rejectUnauthorized: true` and the `session` of an earlier verified connection, `tls.connect({ socket: duplex })` emits `'secureConnect'` with `authorized` true on main, and a wrap emits `'secure'` with `ssl.verifyError()` null here. A write after that fails with `ERR_SOCKET_CLOSED`, and no byte reaches the transport. Reproduction for the first one (needs a key and a certificate, for example `test/js/node/tls/fixtures/agent1-*.pem`): ```ts const server = Bun.listen({ hostname: "127.0.0.1", port: 0, tls: { key, cert }, socket: { data() {}, open() {}, error() {} } }); await Bun.connect({ hostname: "127.0.0.1", port: server.port, tls: { rejectUnauthorized: false }, socket: { data() {}, error() {}, handshake(socket) { socket.setSession(socket.getSession()); /* the process aborts here */ } }, }); ``` Reproduction for the second one: ```js const tls = require("tls"), { Duplex } = require("stream"); let answered = false; const raw = new Duplex({ read() {}, write(chunk, encoding, callback) { callback(); if (!answered) { answered = true; setImmediate(() => this.push(Buffer.from("HTTP/1.1 400 Bad Request\r\n\r\n"))); } }, }); const socket = tls.connect({ socket: raw, rejectUnauthorized: false }); socket.on("secureConnect", () => console.log("secureConnect")); // main prints this socket.on("error", error => console.log(error.code)); // node prints ERR_SSL_WRONG_VERSION_NUMBER ``` **Gaps that this PR does not close.** Each one also exists on main for `tls.connect({ socket })`, with the same result. | shape | node | this PR | owner | | --- | --- | --- | --- | | `end()` or `destroySoon()` before the socket connects | waits for `'connect'`, then sends the FIN | `'finish'` at once, no FIN | #42339 | | refused connection under a wrap | `'_tlsError'`, `'close'` | uncaught `ECONNREFUSED`, the wrap stays open | #38122 | | `end()` over a `Duplex` before the handshake completes | runs the `final()` of the `Duplex` | `'finish'`, no `final()` | #42350 | A wrapped `Duplex` that fails when it is read was in this list. #42235 landed, and the wrap now reports `'_tlsError'` and then `'close'`, as node does (measured on d31efd7). **Inherited options.** `kUpgradeClientTLS` passed a plain `{ socket, servername }` object to `Socket.prototype.connect`, and that function reads `rejectUnauthorized` through the prototype chain. With `Object.prototype.rejectUnauthorized = false`, the wrap accepted an untrusted certificate, also with an explicit `rejectUnauthorized: true`. Commit 6657bc3 passes the decision of the constructor as an own property, as `tls.connect()` does. Measured on this head under that pollution: a wrap with default options, with `{}` and with an explicit `true` rejects, and an own `false` accepts. `new TLSSocket(raw, { rejectUnauthorized: undefined })` with `NODE_TLS_REJECT_UNAUTHORIZED=0` rejects. **`'finish'`.** `new TLSSocket(new PassThrough()).end()` emits `'finish'` and `'close'` on this head. The shutdown cells do not assert `'finish'`. A separate check reports that `'finish'` is lost there when #43962 is applied on top of this PR. This session did not build that combination. Cell by cell for the 21-cell matrix of #42330 on the earlier head: #37664 (comment) **Releases.** Earlier comments in this thread measured the same failures on Bun 1.4.0 and 1.4.3. This session measured main only. **User.** `Connection.prototype._startTLS` in mysql 2.18.1 (`lib/Connection.js`) is the known caller of the client-side constructor. The replay test matches that function line by line, and `lib/protocol/sequences/Handshake.js` sends the SSLRequest and starts TLS with no reply in between. The xmpp report in this thread is for `tls.connect({ socket })`, a different path. A search of the open and closed issues finds no report for the constructor. **Guard design.** #42330 kept the stream as `_handle` and added a guard at 2 of the 6 places that call it as a native handle. **Signatures on main.** `destroy()` on the wrap fails with `handle.close is not a function`. `end()`, `end(cb)` and `destroySoon()` throw `socket.shutdown is not a function` from `process.nextTick`. **Tests.** All are in `test/js/node/tls/node-tls-connect.test.ts`, block `new tls.TLSSocket(socket) on the client side`. - Four reports come from `node-tls-client-wrap-fixture.mjs`. Bun calls its functions in the test process. Node runs the same file as a script. The expected report is the same for both. - `shutdown`: 16 cells. The methods are `end()`, `end(cb)`, `destroySoon()` and `destroy()`. The streams are a connected, a connecting and a never-connected `net.Socket`, and a `Duplex`. Each cell calls the method and then `destroy()`. It asserts no throw, no `'error'` and `'close'`. The cells run together. On main each cell fails with `socket.shutdown is not a function` or `handle.close is not a function`. - `mysql`: the calls of `Connection.prototype._startTLS` in mysql 2.18.1, in the driver's order and at its time. The driver writes the SSLRequest and starts TLS in the same turn, and the server sends no reply in between. Three configurations: `rejectUnauthorized: false`, the CA of the server, no CA. `onSecure` runs one time in each. - `peerCloses`: the peer closes when the ClientHello arrives. - `session`: the `session` option on the three paths, and `setSession()` before the socket connects. Each one resumes. - Both sides of `rejectUnauthorized` run in the test process only, because node accepts in each case. `unlike node, an untrusted certificate destroys the wrap with the verify error` has one case for default options and one for `true`. `with rejectUnauthorized: false, 'secure' fires for an untrusted certificate and a write goes out over TLS` is the other side. `NODE_TLS_REJECT_UNAUTHORIZED=0 turns the default off, as for tls.connect()` pins the environment variable. - `an inherited rejectUnauthorized cannot turn the check of a wrap off` runs in a child process with `Object.prototype.rejectUnauthorized = false`: default options and an own `true` reject, an own `false` accepts. It fails on d31efd7. ``an own `rejectUnauthorized: undefined` still rejects with NODE_TLS_REJECT_UNAUTHORIZED=0, as for tls.connect()`` pins that rule. - `setSession() on a wrap has no effect: it does not abort the process and does not throw` runs in a child process, because the failure is a process abort. Without the check it gets exit code 134. No test covers a late `setSession()` on a `tls.connect()` socket: it aborts the process until #41671 lands. - On main (canary 367d939, release build), 16 of the 21 tests in the block fail. 8 fail at once, and 8 fail by the timeout, because main starts no handshake. The 5 that pass are the http2-wrapper guard and the 4 rows that run node. - The SNI test fails when only the `servername` argument is reverted (`Expected: "sni.example"`, `Received: undefined`). **Cost for callers that never wrap a socket,** from the diff: - per `net.Socket`: one more property store in the constructor. - per TLS `connect()`, per client handshake and per `setSession()`: one more property read and branch. - per `internalConnect` and `internalConnectMultiple`: two property reads and branches fewer. Each `[buntls]` options object has one property fewer. - `tls.connect({ socket })` puts the same bytes on the wire as on main (1452). - This session did not measure instructions, syscalls or binary size: `perf`, `valgrind`, `strace` and `bloaty` are not in the test container. A separate differential check of d31efd7 merged onto main reports equal instruction counts: 10,162,756 (main) and 10,159,922 (this PR) for each TLS connection, and 2,195 and 2,200 for `new net.Socket()`. **Suites run with a debug build of this head.** - On the head a460a9d: `test/js/node/tls/node-tls-connect.test.ts` gives 107 pass, 18 skip, 0 fail with a 30 s limit, in 2 of 2 runs. With the default 5 s limit, 4 to 8 tests reach the timeout in each run on this machine, and the set differs from run to run. Most of them came from main. The load average was 450 to 970 on 16 cores. - One of those tests from main (`server write() and end(data) from inside ALPNCallback`) takes the same time with the source of main and with this PR: 3.9 to 6.4 s and 3.8 to 6.0 s, 10 runs each. - `tsc --noEmit -p src/js/tsconfig.json` and `bun lint` pass on a460a9d. - The suites below ran on the head c81cee3, before the merge of main. - `test/js/node/tls/` (28 files): 2 failures in each run, and this change causes neither. `SNICallback runs even when the requested servername matches the bind hostname` fails on the release build of main too. `concurrent Workers all see the same CA certificate lists` fails 5 of 5 times with the `net.ts` and `tls.ts` of main on the same debug build. In the last run the machine was overloaded, and 2 more tests reached the 5 s timeout. Both pass alone in 3 of 3 runs, and neither builds a client-side wrap. - `test-tls-*`, `test-https-*`, `test-net-*`, `test-http2-*` in `test/js/node/test/parallel` (657 files): no failure from this change. 10 files fail on main too (`test-https-proxy-request*.mjs`, `test-https-request-proxy-post.mjs`, `test-tls-client-allow-partial-trust-chain.js`). `test-https-timeout.js` hangs on a debug build, with the source of main too. - `test/js/bun/net/socket.test.ts`: 94 pass, 1 fail. The failure needs DNS for `www.example.com` and fails on main too. </details>  --- **[human-review]** gate passed · iteration 0 · 5 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 16 failed, 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [3.32ms] (pass) should thow ECONNRESET if FIN is received before handshake [351.98ms] (pass) initializes authorizationError to null in the TLSSocket constructor [9.71ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [175.18ms] (pass) should be able to grab the JSStreamSocket constructor [18.49ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [114.28ms] (pass) tls.connect > should have peer certificate when using self asign certificate [269.78ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port ... (truncated) release without fix: 34 failed, 18 skipped bun test v1.4.3-canary.1 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [20.95ms] (pass) should thow ECONNRESET if FIN is received before handshake [48.67ms] (pass) initializes authorizationError to null in the TLSSocket constructor [0.39ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [5.88ms] (pass) should be able to grab the JSStreamSocket constructor [0.30ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [5.44ms] (pass) tls.connect > should have peer certificate when using self asign certificate [24.26ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port and host correctly (skip) tls.connect > should process port, host, and callback correctly (skip) tls.connect > should handle the absence of a callback gracefully (skip) tl ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [3.27ms] (pass) should thow ECONNRESET if FIN is received before handshake [394.13ms] (pass) initializes authorizationError to null in the TLSSocket constructor [8.43ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [196.56ms] (pass) should be able to grab the JSStreamSocket constructor [33.22ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [298.86ms] (pass) tls.connect > should have peer certificate when using self asign certificate [101.96ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port ... (truncated) release with fix: 18 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision d31efd7 features lto, baseline 23 deps, 136 codegen, 1176 objects in 6018ms ninja: Entering directory `/workspace/bun/build/release' [1/4] fetch lolhtml [lolhtml] up to date [2/4] fetch rust-argon2 [rust-argon2] up to date [2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json 244 units: 172 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib [3/4] reconfigure [1/1499] mkdir stamps [2/1499] mkdir codegen [3/1499] install /workspace/bun bun install v1.4.3-canary.1 (367d939) Checked 26 installs across 65 packages (no changes) [231.00ms] [4/1499] rustc unicode_xid [5/1499] rustc heck [6/1499] rustc build_script_build [7/1499] rustc build_script_build [8/1499] rustc unicode_ident [9/1499] rustc build_script_build [10/1499] rustc build_script_build [11/1499] install /workspace/bun/packages/bun-error bun install v1.4.3-canary.1 (367d939) Checked 1 install across 2 packages (no changes ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/js/internal/net/symbols.ts | 2 + src/js/node/net.ts | 68 ++-- src/js/node/tls.ts | 47 +-- test/js/node/tls/node-tls-client-wrap-fixture.mjs | 387 ++++++++++++++++++++++ test/js/node/tls/node-tls-connect.test.ts | 356 +++++++++++++++++++- 5 files changed, 806 insertions(+), 54 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/js/internal/net/symbols.ts 0 0 72 src/js/node/net.ts 8 0 73 src/js/node/tls.ts 1 0 74 test/js/node/tls/node-tls-client-wrap-fixture.mjs 2 3 77 test/js/node/tls/node-tls-connect.test.ts 0 0 69 ``` </details>

Jarred-Sumner · 2026-10-06T06:19:53Z

Superseded by #44618, which consolidates the open TLS pull requests. This fix and its tests are in there, either as written, rewritten smaller, or merged with the other PRs that patched the same cause (see the "By area" list in that PR). Closing in favor of it.

…t when an engine is attached (#38028, #38122, #38076) Node ties the two sockets together in the TLSSocket constructor, whatever state the wrapped one is in: its 'close' destroys the wrap, its 'error' is re-emitted, and closing the TLS handle destroys it. Bun made those links at the moment a TLS engine was attached, in six copies, and the last one only for a generic Duplex or an adopted fd. So a wrap with no engine yet (a client waiting for 'connect', the server's one-tick deferral, a server over a connecting socket) and every stream-level wrap over a net.Socket had none: the fd leaked and server.close() never called back, a refused connect was uncaught or reported nowhere, a socket dialed again was upgraded by its dead wrap, and a server wrap adopted a half-connected socket, which kept the process alive. linkUpgraded() makes the links once, before any branching. The client and server attach code is one function each over two shared helpers. _destroy takes down any transport that is not the raw twin of an adopted fd, where node does it, next to the handle close, so it is still intact inside 'error'. A server wrap over a connecting socket waits for 'connect', and named pipes take the stream-level engine there as they do on the client. The constructor refuses a secureContext that is not one (ERR_TLS_INVALID_CONTEXT) instead of throwing out of nextTick.

…ps, WebSocket, SQL) (#44618) ### What does this PR do? Consolidates the open TLS pull requests into one. Each was reproduced on `main` and, for `node:*` behavior, on Node v26.3.0 first. About a third are ported as written, the rest are rewritten smaller or merged into one fix where several PRs patched the same cause. One commit per fix, so it can be read commit by commit. Fixes #43520, fixes #31396, fixes #43635, fixes #37193, fixes #43846, fixes #17932, fixes #41061, fixes #36887, fixes #31810, fixes #35240, fixes #32234, fixes #44365, fixes #43807, fixes #42280, fixes #44517. Addresses #41856 (SNI and `servername`; not `checkServerIdentity` for SQL), #24845 (the spin is gone, shown with fault injection on Linux; not run on macOS), #19754 (node-fetch forwards the agent's TLS options; the Kubernetes client itself was not run). #### The ones that matter most | | On `main` | PRs | |---|---|---| | Client certificate disclosure | `https.request()` with a client certificate sends it to a server it then refuses (wrong name, `checkServerIdentity`, `destroy()` in `'secureConnect'`, `terminate()` in `handshake`). A server can force it with a junk record behind its Finished | #43946 | | False `authorized` | Over a Duplex, `secureConnect` with `authorized === true` for a peer that failed the key proof; `secureConnect` for a plaintext peer with `rejectUnauthorized: false` | #44422, #32929 | | Cleartext https | `https.createServer()` without a usable key/cert answers plain HTTP | #41672, #33539 | | Revoked client certificates | An https mTLS server never sees `crl`, so a revoked client is `authorized` | #41641 | | Pooled sockets | Requests with different client certificates or CAs share an `https.Agent` socket and session | #42498 | | Silent plaintext | `tls: [...]` given to `Bun.listen` / `Bun.connect` is plain TCP | #41490 | | Server weakened by a client knob | `NODE_TLS_REJECT_UNAUTHORIZED=0` turns off a server's client-certificate enforcement | #35245 | | Pins never checked | `WebSocket` never calls `tls.checkServerIdentity` and ignores `tls.serverName` | #41648 | | `verify-full` dropped | `PGSSLMODE=verify-*` is lost next to a `TLS_*` URL variable; `tls: true` sends no SNI | #44498 | | Crashes | use-after-free from `destroy()` in `ALPNCallback` over a Duplex; `abort()` on a late `setSession()`; SIGABRT in `fetch` with an https proxy from the environment and a `Bun.file()` body | #44462, #41671, #44458 | | Stream corruption | A TLS `write()` can lose 16 KiB it reported as written while another socket on the loop is stalled | #44529 | | Hangs and spins | 100% CPU on a failing `send()`; a fatal `SSL_write` leaves the socket open forever; `idleTimeout` never sheds a TLS client that ignores `close_notify` | #34510, #38176, #42336 | | Wrong certificate (regression since 1.3.14) | Connections accepted before `stop()` / `close()` get the default certificate and skip their entry's `requestCert` / `ca` | #42355 | | Quadratic Duplex / proxy tunnel | Reading one chunk over a Duplex, CPU: 8 MB 0.88 s → 0.14 s, 16 MB 3.18 s → 0.23 s, 32 MB 11.75 s → 0.39 s; `fetch` upload through CONNECT: 1.7 s → 0.18 s (debug build) | #44464 | #### By area - **fd engine, write path** (`openssl.c`, `socket.c`): #42352, #34510 + #38176 + #42336 as one change, #44529, #44458, #44192. A rejected `send()` ends the write side only and closes at the next writable event unless the peer's bytes are still queued (a 413 sent before a reset is still read). No new per-socket state. Also, on kqueue, **a FIN no longer ends a socket that waits in the low-priority queue** (`loop.c`): with more than 5 TLS handshakes at once, a client that ended right after its handshake could be reset and its server socket report `socket hang up`, because the eof that the sentinel read knote reports was acted on ahead of the unread Finished. That is on `main` too (the macOS entry for `node-tls-server.test.ts` in `test/flaky-tests.txt`: 7 of 48 recent builds of other branches), and this branch made it likelier (6 of 8 builds), since Finished now leaves in one segment with the close_notify. - **Error reporting, both engines**: #44422, #32929, #44516, #37094, #41272 + #42324 + #44223 as one change, #44021, #37472, #43946, #33630. One channel: a fatal error on an established session is reported, then **the engine closes the connection itself**, whatever the owner does with the report. `test/js/bun/net/tls-fatal-error-closes.test.ts` asserts closed-and-nothing-delivered for every owner (node:tls, `Bun.connect`, `Bun.listen`, `fetch` direct and through CONNECT, `Bun.serve`, `WebSocket` direct and through a proxy, Postgres, MySQL, Valkey, Duplex). - **Duplex engine** (`SSLWrapper`, `UpgradedDuplex`): #44462, #43529, #42332, #44464. #43877 + #44394 were in and are **out again**, see "Worth a look" 5. - **node:tls wrap lifecycle** (`net.ts`, `tls.ts`): #38007, #38058, #38028 + #38122 + #38076 as one change (six copies of the attach code become two helpers), #38311, #39008, #38154, #42340 + #42343 + #42339 + #42453 as one change, #43791, #42425, #44085, #42683, #39088, #39040, #40375, and what was still real of #36534. - **SNI, ALPN, server contexts**: #43080, #42050, #37195 + #43849 as one change (**one** SNI matcher for TCP and HTTP/3), #42355, #42285, #33253, part of #37896, part of #37013. A `tls.Server` has one `SSL_CTX`. - **Verification and options**: #44738, #41490, #37005 + the cwd pin of #40984, #31811, #43982, #33483 + #35245, #41810, #32235, #44441, #38092. - **node:tls API and CA store**: #41671, #38145, #32824, #43594, #39997, #41696, #33534, #34748, #42991, #42996, #42970. - **node:https, Agent, `ws`, node-fetch**: #41672 (https half), #41641, #38261, #42498, #44346, #35609, #31397, #42325. - **WebSocket client**: #41648, #37487 + #43048 as one change. - **SQL, Redis**: #33666, #41711, #44498, part of #42054. - **Tests only**: #41426, #40040, #44395, #44016, #37860, #40591, #44440, #41424. Found on the way and fixed here: an upload that a TLS 1.2 server interrupts with a renegotiation never completes on `main` (0 of 32 runs over `https.request`, `fetch`, `node:tls` and `Bun.connect`: the renegotiation ClientHello lands inside an application record that is still unsent, or the socket gets no `drain` again) and completes here, with two tests from robobun; the fix for #40653 (final flight and first write in one segment) stopped working whenever another TLS socket on the loop was stalled, on `main` too; the `tls.Server` prototype pinned the last server constructed and every `SSL_CTX` it owned; `Object.create(process.env).NODE_TLS_REJECT_UNAUTHORIZED = "0"` turned verification off process-wide once a `SHARE_ENV` worker existed; two debug panics when wrapping a shut-down or still-connecting socket; a `fetch` POST through a proxy sent its headers twice when the origin renegotiated; `BlockList` ignored IPv6 zone ids; a test now ties `root_certs.der` to `certdata.txt`. #### Behavior changes - **A server's `ca` without `requestCert` no longer asks for a client certificate** (`Bun.serve`, `Bun.listen`, HTTP/3, node:tls). It matches the docs and Node. On `main` such a server refused clients with no certificate but served any unrelated self-signed one, so it was never authentication. **Set `requestCert: true` to require a certificate.** A matrix test pins that `requestCert: true` still refuses no certificate and an untrusted one on 8 kinds of server, TLS 1.2 and 1.3, with `NODE_TLS_REJECT_UNAUTHORIZED` unset and `0`. - `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer relaxes a server. - `Bun.connect` / `Bun.listen` hear of a fatal TLS error after the handshake through `error(socket, err)`. With no `error` handler the socket just closes. - HTTP/3 server names match like TCP: `*.` covers exactly one label, case is ignored, a trailing dot is ignored, the last registration of a name wins. - `requestCert` on node:https is `=== true`, as in Node. - An array where a generated options dictionary is expected throws (`tls: []`, `jest.useFakeTimers([])`). - `key` / `cert` arrays serve every identity. A client that can use both gets ECDSA, where `main` served whichever pair came last. - `ecdhCurve` is forwarded by node:https, `ws` and node-fetch now, so a group BoringSSL lacks (`X448`) throws there as it already does in `tls.createServer`. - A wrapped socket's error is re-emitted on the TLS socket as in Node, so `raw.destroy(err)` with a listener on `raw` only is uncaught, as in Node. - `sql.options.tls` is always an object, never `true`. `RedisClient` sends SNI. - `tls: { secureContext }` alone asks for TLS on `Bun.listen` / `Bun.connect` (it was plain TCP), and a value that is not a `SecureContext` throws. The context is served as it is: the `requestCert` / `rejectUnauthorized` it was created with hold whatever the options next to it say, and `requestCert` in the options over a context that does not ask throws at `listen()`. - `tls.DEFAULT_CIPHERS` reaches every client once assigned (`fetch`, `WebSocket`, `Bun.connect`, `RedisClient`, `Bun.SQL`, `S3Client`, proxy tunnels) and servers again. A list that selects no cipher throws `ERR_SSL_NO_CIPHER_MATCH` at the assignment. `fetch.preconnect()` dials nothing after an assignment. - The warning for an unreadable `NODE_EXTRA_CA_CERTS` is Node's one line, without the `warn:` prefix. - `BUN_CONFIG_WS_CLOSE_TIMEOUT` (default 30 s): how long a `WebSocket` client waits for the server to close the connection after the closing handshake. #### Worth a look in review 1. **#44529**: the kernel-refused remainder of a TLS write moves from the loop's one slot onto the connection (in the existing rare struct), so the write BIO never refuses a sealed record. Nothing is allocated on an unstalled path (200 writes: 0 appends, same `send()` count as `main`), memory with 16 stalled writers is lower than on `main` (276 KB vs 340 KB, which `main` holds inside BoringSSL's buffers), `us_socket_t` stays 80 bytes. It needs a bound on how long a deferred close waits, or a peer that stops reading pins the fd past `destroy()`: `US_SSL_CLOSE_AFTER_SPILL_TIMEOUT` is a fixed 10 s, not re-armed on progress. Separate commits, but the fix that keeps the client certificate off the wire beside a stalled socket builds on them. 2. **The default name check of node:tls also runs inside the handshake**, so a wrong-name server gets no client certificate on TLS 1.2 either. JS still runs it after every successful handshake, so a difference between the two matchers can only refuse. Error objects are byte-identical. 3. **#44441** widens trust by design: a self-issued leaf whose `keyUsage` lacks `keyCertSign` (`dotnet dev-certs`) is its own anchor when the store holds a byte-identical copy. No BoringSSL change. Expired pin, same subject with another key, wrong EKU and a pinned intermediate are tested to fail. 4. **#32235** only adds Ed25519 and ECDSA P-521 to the verify list. A captured ClientHello shows `main`'s list with the two inserted; `rsa_pkcs1_sha1` stays. 5. **A stream that a TLS socket wraps, when that TLS socket closes.** An earlier state of this branch lost data here while CI was green (found by #44709's report): with the peer closing first, 4 of 8 MiB arrived with TLS in TLS, 4 of 32 MiB on the http2 `emit("connection")` path, and a `write()` with no `'error'` listener ended the process. Three Node-parity changes only hold together: destroying the wrapped stream at the close (#38028 + #38122 + #38076, #38154) is safe only if every write has really completed (#43877), which in turn needs Node's handling of the peer's close_notify, which needs half-open sockets that the GC can collect. So: - #43877 + #44394 are reverted and reopened. A write over a stream completes once the stream has taken the ciphertext, as on `main`. - Until the verdict on the peer lets the session through, the application cannot have written over it. There the wrapped stream is destroyed as in Node, with the sessions below it. That keeps the release of the connection after a failed handshake, a rejected certificate and an early `destroy()`. The same for an http2 socket the application never got, and for `resetAndDestroy()`. - After that it is `main`'s teardown: a `net.Socket` only gets the engine's `end()`, closes at its peer's FIN, keeps its own timeout and reports its own errors. Any other stream is destroyed with the TLS socket. The regular suites cannot see any of this (999 files were green on every broken variant), so it was steered by eleven seeded differential fuzzers run on this build, `main`, Node v26.3.0 and the earlier state: close, `end()`, `destroy()`, `destroySoon()`, resets, hung and half-open peers, paused writers, timeouts, two and three sessions deep, over TCP and over Duplexes, before, at and after the handshake, and http2 requests. See "How did you verify". #### Known limits - `fetch` with a `checkServerIdentity` function still sends the client certificate (not the request) to a server the function refuses. On TLS 1.2 any verdict a JS callback gives is too late, as in Node. - `addContext()` / `SNICallback` still do not apply to a server-side socket on the stream engine (`emit("connection", duplex)`, TLS in TLS, unflushed writes, named pipes), as on `main`. - A CA bundled in a pfx extends an explicit `ca` only, for `ws` / node-fetch / `WebSocket`: the native `ca` can only replace the default store, and that store keeps `SSL_CERT_FILE` / `SSL_CERT_DIR`. - P-521 leaves work on TLS 1.3 only. TLS 1.2 needs secp521r1 in every ClientHello (`it.todo`). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch(url, { protocol: "http3" })` is `HTTP3Unsupported`, as with an explicit `ciphers`. - `addCACert()` by hand does not extend the chains of a context with several identities. - A throwing `ALPNCallback` sends `no_application_protocol` on both engines. Node sends nothing and its client sees `ECONNRESET`. - TLS in TLS, peer FIN while the outer handshake runs: the inner socket gets one `write EPIPE`, where Node gives `ECONNRESET` (`main` gives it no error at all). - `@SECLEVEL` in `ciphers` is dropped by the `ws` / node-fetch shims, which used to ignore `ciphers`. node:tls keeps throwing `ERR_SSL_INVALID_COMMAND`. - Beside a stalled TLS socket only the first record (16 KiB) of the first write leaves with the handshake flight. The rest goes record by record, which is what bounds the memory of stalled writers. - After a fatal error on an established session the socket emits `'error'` and then `'close'`. Node emits `'error'` and leaves the socket open. - A paused reader whose own write the kernel rejects loses what it had not read yet, with an `EPIPE`, as on Node. `main` reports no error there and delivers it. - On `main` too: a `Bun.listen` socket without `allowHalfOpen` that has unsent ciphertext when the client's `shutdown()` arrives loses that ciphertext (32 KiB), and over plain TCP `end()` with the peer still sending is a close over unread input, so a reset. - Differences from both `main` and Node that the differential runs below found and that stay, all with a peer that aborts: `ECONNRESET` instead of a clean `'end'` after the socket's own `'finish'` when the peer destroyed with unread data; under TLS 1.2, a zero-length `write()` followed by `destroy()` in `'secureConnection'` leaves the client without `'secureConnect'` (a plain `destroy()` there matches Node); a TLS 1.2 client that destroys in `'secureConnect'` gets no `'session'`; a `ClientRequest` whose handshake fails with an alert emits `'error'` and `'close'` but no `'finish'` (`writableFinished` is true). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch.preconnect()` opens nothing: `fetch()` then uses a context of its own, and a socket warmed under the default one would never be picked up. - A TLS `send()` that the kernel refuses outside a `write()` call (the drain of unsent ciphertext) is reported with the close, as `read EPIPE` / `read ECONNRESET`. Node says `write EPIPE`. `main` does not report it at all. - Once the application has a session over a `net.Socket` (TLS in TLS, http2 `emit("connection")`), a peer that never sends its FIN holds that socket after the TLS socket closed, as on `main`. Node destroys it. Two tests of #38154 are `todo` for this. Closing it any earlier (at its `'finish'`, say) makes the kernel drop what it has not sent yet as soon as the peer's close_notify arrives. - Plaintext that was queued on a socket before it was wrapped (STARTTLS with a backlog) is dropped when the TLS socket is destroyed, or its handshake fails, before the session is accepted. Node drops it too, except on `destroySoon()`. `main` sends it. - Over a stream that is no `net.Socket`, `end()` can still cut what that stream has buffered, and there is no backpressure, both as on `main` (#43877). - `tls.secureContext` (the undocumented door node:tls uses) is not read by a Windows named pipe listener, which builds its context from the options. On `upgradeTLS({ isServer: true })` the options next to it are the policy, as with Node's `SetVerifyMode`. - `selectServerName()` rebuilds the name tree per ClientHello for injected sockets of a server with `addContext()` entries: 0.4 µs for 1 entry, 3.7 µs for 10, 41 µs for 100, against 631–1111 µs for a handshake. #### Not included Left open, because they need a decision or are not TLS: #43877 + #44394 (see "Worth a look" 5; #43874 stays open with them), #38548, #38591 (both shrink who is trusted), #41589 (`verify-full` vs `NODE_TLS_REJECT_UNAUTHORIZED=0`), #37197, #41706, #43216, #33487, #33545, #36707, #32435, #37255, #28691, #40275, #30314 (features), #38120 (needs the BoringSSL fork, as did #33517, which the stale bot has closed since), #38529 (needs a Windows measurement), #34342, #38232, #43089, #44454, #40451, #42710, #44527, #38088, #38093, #41898. #37896, #42054 and #37013 stay open for the halves not taken. `http.createServer({ key, cert })` keeps serving TLS on purpose. One open question: `tls: {}` (an object that names no TLS option) is plain TCP on `Bun.listen` / `Bun.connect`, here and on `main`. It is the same trap as `tls: []`, but changing it changes a Bun default, so it is left alone. ### How did you verify your code works? - Every new test fails on `main` for the stated reason and passes here, except guards that pin existing behavior, each shown to fail when its clause is removed. `node:*` tests also pass on Node v26.3.0; the few that cannot say which Node version has the behavior. - 212 test files that touch TLS, sockets, http, http2, fetch, WebSocket, SQL, Valkey and workers: 6154 pass, 2 fail. Both are seen on `main` too: `serve.test.ts` "root range port" (the box runs as root), and `worker_threads.test.ts` "terminate(): nothing of the worker's runs after the request", which is flaky there and passed in the run below. - 58 of those files the way the ASAN lane runs them (LeakSanitizer + `BUN_JSC_validateExceptionChecks`): 58 files, 48 of them with leak checking, 4132 pass, 3 fail. All three also fail on `main`: `serve.test.ts` "root range port", `node-net.test.ts` "should not leak when connect({path}) fails synchronously on a reused handle" (times out under this environment), `worker_threads.test.ts` "process.exit() with a shell cp in flight" (a `ShellCpTask` leak). - 647 vendored `test-tls-*`, `test-https-*`, `test-net-*`, `test-http2-*`: the only two failures also fail on `main`. - The SNI matcher was diffed against both old matchers: 3 seeds × 1.23 M lookups × 3 registration flavours, every difference in one of the intended classes, TCP and HTTP/3 identical on every lookup. - The headline rows were also driven by hand with scripts against this build, `main` and Node v26.3.0: cleartext https, `crl`, `tls: []` / `{ secureContext }`, the `ca` / `requestCert` matrix, the client certificate on a wrong-name server, late `setSession()`, `destroy()` in `ALPNCallback`, `[rsa, ec]` identities with an intermediate from `ca`, `WebSocket` `checkServerIdentity`, a corrupted record, the Duplex read above, `tls.DEFAULT_CIPHERS`. - The `setSession()` guard was checked against the real `abort()` at 43 handshake states. - `bun run rust:check-all`: 12 of 12 targets. `tsc`, oxlint, source lints, prettier, rustfmt, mordant clean. - usockets' `_Nonnull` is compiled out of debug builds, so 105 of those files were also run on a local release ASAN build with the CI runner's environment (92 with leak checking): 4595 pass, 1 fail, `child_process.test.ts` "spawn reports EPERM after dropping privileges", which cannot pass as root and fails on `main` too. - The close of a TLS socket over another stream ("Worth a look" 5): eleven seeded differential fuzzers, 8,424 scenarios compared, each run on a release ASAN build of this branch, on `main`, on Node v26.3.0 and on the earlier state of the branch. Against `main`: - Data that `main` delivers in full is cut in 5 scenarios, and about 150 that `main` cuts arrive in full. Of the 5, in 2 `main` never notices the peer's close and keeps the socket for good, 2 call `end()` on the middle one of three sessions over an in-memory Duplex, and 1 does the same on Node. - No dead timeout, no silent reset and no uncaught error that `main` does not have (4 uncaught errors fewer). - A socket stays open where `main` closes it in 109, and closes where `main` keeps it in 295. 92 of the 109 do the same on Node or on the earlier state (a `destroy()` that an in-memory Duplex does not show its peer, half-open peers). 14 wait for a peer that paused reading and so does not read the FIN (#42332's backpressure, as in Node); the socket's own timeout fires there. 3 are left: one on a 5 ms timer, two with three sessions over an in-memory Duplex. - The earlier state of the branch cut data in 173 of the 400 scenarios of one of them, where `main` cuts none and this cuts none. - 23 new tests pin what they found. Each earlier attempt at this fix fails the ones that describe it, the earlier state of the branch fails 7, and all pass on Node. - After that change: 999 test files on the release ASAN build (20,246 pass; the 11 files that fail need a database, Docker, DNS or a non-root user, or share a temp directory with a parallel run and pass alone), 61 on the debug build. - TLS over a file descriptor (`openssl.c`, the path of `fetch`, `Bun.serve`, `tls.connect`, `Bun.connect`) got the same treatment after the rebase: seeded differential fuzzers on CI's release build of this branch, on `main` and, for `node:*`, on Node v26.3.0. Every runtime also against itself for the noise floor, injected faults and known bugs of `main` as positive controls, and a difference counts only if it shows in 5 of 5 fresh processes. - `node:tls` over TCP: 11,500 scenarios (one connection with Node as the oracle line by line; 2 to 60 connections beside stalled neighbours; raw peers that break the handshake). HTTPS: about 136,000 runs over `Bun.serve` + `fetch`, `node:https`, `node:http2` and `wss://`, also with the two ends in different runtimes. `Bun.connect` / `Bun.listen` / `upgradeTLS`: 11,500 scenarios and 720 slow connections, with writers driven by what `write()` returns, beside up to 6 stalled, dripping, closing or resetting neighbours, and plain TCP as a second oracle. No crash, hang, duplication, reordering or silent truncation, and no change in time or in connection reuse. - They found six things that `main` does better, none of which any test showed. All are fixed, each with a test that fails on the build before: what the peer sent lost behind a rejected `send()` (23 scenarios, and an early HTTPS response lost with only `EPIPE`), the same silently for a paused reader, `server.close()` never calling back on a half-open server after a ClientHello and a reset (17), `closeAllConnections()` taking 12 s with a stalled client, `end()` losing up to 1.3 of 4 MiB that `write()` had reported while the peer still uploads, and `end()` a little after a stall never closing beside other stalled TLS sockets. The last two fixes also deliver the 1 to 2 MiB that `main` loses there, and close the socket that `main` keeps for good without such neighbours. - All of them again after every fix, on CI's release build of it. That caught one regression of a fix itself (a reader stopped for backpressure lost 86,385 bytes, 1 of 6,000 scenarios), fixed too. On the last build: scenarios that lose data where `main` does not 23 → 2, and Node loses it in both, with the same `EPIPE`; `server.close()` that never calls back 17 → 0; connections held 4 → 0; requests that end in an error only where `main` has a response 6 → 0. With a Node server in another process, a request ends in an error only in 8 and 10 of 1,500 scenarios here, 5 and 3 on `main`, 10 with Node as the client. - `Bun.connect` / `Bun.listen` on the last build against `main`, in scenarios: hangs 0 against 1,031, sockets and fds never released 0 against 965, corrupted data 0 against 345, `abort()` 0 against 26 (`setSession()` after the handshake), writers that never close 0 against 101 of 720 connections. No kind of failure shows here and not on `main`. About a third of the slow connections close later than on `main`, in 1 to 16 s instead of at once, waiting for unsent ciphertext or for the peer's close_notify, and 79 more of them deliver all that `write()` reported. RSS and time with 16 to 256 stalled writers are the same. - What they found that `main` does worse: a `WebSocket` that calls `close()` with sends pending loses messages in 81 of 999 scenarios (0 here), 37 server sockets left open, 10 `server.close()` that never call back, 20 write callbacks that never run. - The kqueue fix cannot be run on Linux. The `connectionListener` count test now says what became of a missing connection, which is how the cause was found (`'tlsClientError'` "socket hang up", then `read ECONNRESET` at the client of the same port, after its `'secureConnect'`). On macOS x64 it failed every attempt of the three builds before the fix and passed at the first attempt of the build with it. - Windows and macOS were only run by CI. Four new tests asserted what only the Linux kernel does (a FIN read ahead of a reset, unread bytes surviving a reset, loopback buffer sizes, `fstat()` on a socket) and now say so per platform. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>

github-actions Bot added the claude label Aug 13, 2026

claude Bot reviewed Aug 13, 2026

View reviewed changes

test: cover reconnecting the wrapped socket after the failed connect

e1cf627

github-actions Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread src/js/node/net.ts Resolved

github-actions Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread src/js/node/net.ts Resolved

net: shorten the comments on the tls wrap's wait-for-connect listeners

9c8e832

claude Bot reviewed Aug 13, 2026

View reviewed changes

This was referenced Sep 22, 2026

node:tls: report a net.Socket transport's error on the TLS socket #43791

Closed

node:tls: make client-side new TLSSocket(socket) upgrade the socket it wraps #37664

Merged

robobun mentioned this pull request Oct 1, 2026

node:https: handle invalid TLS options in proxied requests #44346

Closed

Jarred-Sumner mentioned this pull request Oct 6, 2026

tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618

Merged

Jarred-Sumner closed this Oct 6, 2026

Navigation Menu

Repository navigation

node:tls: report a wrapped socket's connect failure on the TLSSocket - #38122

node:tls: report a wrapped socket's connect failure on the TLSSocket#38122
robobun wants to merge 3 commits into
mainfrom
farm/6e549946/tls-connect-socket-connect-failure

robobun commented Aug 13, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 13, 2026 •

edited

Loading

Review limit reached

Uh oh!

robobun commented Aug 13, 2026 •

edited

Loading

Uh oh!

robobun commented Aug 13, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

Jarred-Sumner commented Oct 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Uh oh!

coderabbitai Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Jarred-Sumner commented Oct 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Aug 13, 2026 •

edited

Loading

coderabbitai Bot commented Aug 13, 2026 •

edited

Loading

robobun commented Aug 13, 2026 •

edited

Loading

robobun commented Aug 13, 2026 •

edited

Loading